-
Notifications
You must be signed in to change notification settings - Fork 95
feat(claude): remove coder_report_task tool call message #157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
✅ Preview binaries are ready! To test with modules: |
mafredri
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, this will remove tool calls entirely from message history as well? Would it be valuable to keep them around, but as a different type of message, or under a new endpoint?
For debuggability, I could see them being useful.
Makes sense to keep them around for debugging, imo we can just log them for now - this should be enough for our usecase as the logs are stored in |
Reasonable 👍🏻 |
mafredri
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last concern, but otherwise LGTM 👍🏻
|
|
||
| if strings.HasPrefix(line, "● coder - coder_report_task (MCP)") { | ||
| toolCallStartIdx = i | ||
| } else if toolCallStartIdx != -1 && strings.HasPrefix(line, "●") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible that ● coder - coder_report_task (MCP) is the last entry, and there won't be a next message with ●? In which I assume we'd remove all the remaining lines?
Asking because I don't see handling for this case (unless I'm just missing it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible that ● coder - coder_report_task (MCP) is the last entry, and there won't be a next message with ●? In which I assume we'd remove all the remaining lines?
@mafredri
Added the logic to handle this case. This handles the case where this is the last thing claude outputs is the tool call, and it also acts as a failsafe if the next new message is not prefixed with ●.
I'm checking for a newline after the tool-call and not directly using len(lines) in above case, because I never want to run into this: In which I assume we'd remove all the remaining lines?
|
Thanks for the quick fix here Jay. |
Closes: #155